Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: merge group conds clause #7198

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

a631807682
Copy link
Member

  • Do only one thing
  • Non breaking API changes
  • Tested

What did this pull request do?

https://gorm.io/docs/advanced_query.html#Group-Conditions

Since it is not safe to reuse a db instance, group conditions will generate duplicate where conditions when used in scope.

User Case Description

refer to #6148 (comment)
close #7136

}

for _, tx := range txs {
stmt := tx.Statement
Copy link

@cbaker cbaker Sep 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it work if we make a function func (db *DB) mergeClause(tx *DB) and then call it in the loop on line 397 to cut down on iterations? db.mergeClause(scope(cleanDB))

Copy link
Member Author

@a631807682 a631807682 Sep 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is to avoid the impact of multiple AddClause on the db instance, so a clean stmt needs to be cached (Clauses are not affected by AddClause)
At the same time we need to keep the order of conditions, such as .Where(...).Scope(...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Grouped conditions in scopes results in duplicative where conds
2 participants